Skip to content

feat(harness): compile focused context [Agent Map 11/15] - #830

Merged
ynadge merged 1 commit into
mainfrom
review/agent-map-11-focused-briefs
Sep 6, 2026
Merged

feat(harness): compile focused context [Agent Map 11/15]#830
ynadge merged 1 commit into
mainfrom
review/agent-map-11-focused-briefs

Conversation

@ynadge

@ynadge ynadge commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Primary change type

  • Feature

Problem and motivation

A delegated assignment needs bounded relevant project context while retaining the same tools and write authority as an ordinary session.

Summary and scope

Compile deterministic canonical and ad-hoc briefs, evaluate targeted impact, retain immutable refresh history, and project bounded context. Refresh follows accepted plan writes without rolling them back on compiler failure.

Reserve a trusted automatic-refresh receipt namespace that caller request IDs cannot occupy. Return precise refresh recovery guidance and preserve the original selection indices in diagnostics.

How this increment fits

Focused context affects prompt content without changing authority or tool access. A failed brief refresh cannot roll back an accepted plan write.

Stack and review boundary

  • Part 11 of 15 in the Agent Map review stack; review this increment against its predecessor.
  • Base: review/agent-map-10-plan-tools.
  • Current head: 74884b1434c1e725ce7ee951e57bd241542f4967; 2,478 changed lines across 22 files, counting additions and deletions including tests.
  • Repackages the corresponding final behavior from #807. Original code and review history remain preserved.
  • Complete coworker testing branch: fix/studio-onboarding-followups.
  • The stack remains unmerged. Dependent PRs target their predecessor, so their diffs do not repeat earlier increments.

Related work

Agent Map checkpoint SAP-3147; relevant work SAP-3150. This packaging follows the maintainer-approved 15-PR split.

Validation

Root checks ran against 662edabbed10dd40d95cf9fbd99c75005984b130. The final head changes only README terminology or commit ancestry; a complete tracked-file comparison confirms identical executable source and build inputs. The terminology gate was rerun on 74884b1434c1e725ce7ee951e57bd241542f4967.

pnpm build — passed (exit 0)
pnpm typecheck — passed (exit 0)
pnpm lint — passed (exit 0)
pnpm test — passed (exit 0)

Tests and documentation

Regression coverage: Golden deterministic compilation, canonical and ad-hoc scope selection, impact evaluation, bounded context, immutable refresh history, reserved request IDs, and focused create/resume.

See part 15 for integrated browser, native CLI, and Mac journey validation. The checks above were run independently on this PR’s own commit.

Linux tests run with ordinary user filesystem permissions; the sandbox's extra ambient capabilities are dropped. Hosted CI and automated review are separate from these recorded local results.

Compatibility and release impact

  • Compatibility: Adds public compiler, brief service, impact evaluator, and bounded context projection helpers. Existing unfocused prompts remain unchanged.
  • Changeset: Included: .changeset/focused-project-briefs.md

Security

  • No secrets, credentials, private user data, or unsanitized logs are included.
  • This PR does not publicly disclose a suspected vulnerability.

AI assistance

  • Codex assembled the implementation, addressed reproduced defects, supplied tests and documentation, inspected the diff, and ran the checks above. Reviews are handled by hosted PR automation.

Checklist

  • Read CONTRIBUTING.md; implementation follows the requested 15-PR split.
  • Description reflects this PR's actual predecessor-relative diff.
  • Relevant tests accompany the changed behavior.
  • Root build, typecheck, lint, and test evidence matches the final implementation; any documentation-only update is identified above.
  • Release/documentation treatment is explained above.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Review — PR #830 (Agent Map 11/15, focused context)

No confidentiality issues: the changeset, docs/shared-build-plan.md, tests, and the
stock-research-compile.golden.json fixture are role-neutral, digest-only, and name no company.

Findings

1. Derived brief requestId shares a namespace with caller-supplied ones — permanently poisonable

agent-map-mcp-tools.ts:279 and :300 derive requestId: \brief-${result.plan.versionId}`, but build_plan_brief_refreshlets the caller pass anyopaquestring asrequestId (build-plan-schema.ts:55` — no reserved-prefix check). Reachable sequence, all from one session:

  1. Plan is at planv_X. Agent calls build_plan_brief_refresh with
    requestId: "brief-planv_X" and focus: {mode:"focused", …} → receipt stored under that id
    with the focused-mode request digest.
  2. Agent calls build_plan_apply with unchanged content → noOp, result.plan.versionId is still
    planv_X (build-plan-service.ts:446,584) → derived requestId is brief-planv_X, focus
    canonical → different requestDigestAgentBriefService.replay throws request_id_reused
    (agent-brief-service.ts:234).

The catch turns that into briefRefresh: { outcome: "retryable" }, and every later no-op
apply/rebase at planv_X fails identically, so canonical briefs never refresh for that plan
version. Version ids are handed to the agent in every tool result, so this is not an exotic input.
Fix: namespace the derived id out of the caller's reach (e.g. reject a caller requestId matching
^brief-planv_, or derive from a digest the caller cannot reproduce).

2. briefRefreshFailure reports non-retryable errors as retryable

agent-map-mcp-tools.ts:128-135 maps everything except quota_exceeded to retryable. The
standalone tool's own errorResult (:105-110) correctly maps request_id_reused/
request_id_expirednew_request, source_mismatchreread, malformed_inputcorrect.
So the same error class gets contradictory recovery advice depending on the entry point, and an
agent that obeys retryable re-runs apply — which regenerates the identical deterministic
requestId and fails the same way. Reuse the errorResult recovery mapping here.

3. Public surface widened at minor with a path consumers cannot use

src/index.ts now exports AgentBriefService, DeterministicAgentBriefCompiler,
compileAgentBriefs, compileCanonicalWorkstreamBriefs, projectFocusedBriefs,
evaluateAgentBriefImpact, serializeFocusedSessionContext, FocusedSessionContextProjection
and 5 constants. Nothing in the repo imports any of them from the package — not the CLI, not
harness-desktop, not web/src (only public-build-plan-entrypoint.test.ts touches one
constant). More concretely, the workflow docs/shared-build-plan.md documents — "passing the
branded projection through TrustedSessionCreateOptions or TrustedSessionResumeOptions" — is
unreachable: SessionManager and both option types are not exported from src/index.ts
(only ensureSpawnHelperExecutable is, index.ts:90). So the branded type ships publicly while
the only API that accepts it does not. Either export the consuming surface in the same increment
or keep the compiler/service/serializer internal until the increment that activates them; every
name here is a permanent contract once published. compileAgentBriefs is additionally a
third alias for projectFocusedBriefs (agent-brief-compiler.ts:719) with no prior published
name to stay compatible with — drop it. The changeset should also list the added exports
(harness CLAUDE.md checklist).

4. Compiler diagnostics point at the wrong selections[i]

agent-brief-compiler.ts:572 iterates sorted(request.selections, …).entries(), then emits
selections[${selectionIndex}].focusScope (:576) and selections[${selectionIndex}].mission
(:639) using the sorted index. A caller submitting selections in any order other than
scope-key order gets a diagnostic path naming a different selection than the one that failed.
Carry the original index alongside the sorted entry.

Verdict

Request changes: fix (1) and (2) before merge — both are agent-facing correctness bugs in the new
build_plan_apply/rebase refresh path. (3) is the call to make now rather than after publish.
Test coverage for the compiler, projection redaction/escaping, quota exhaustion and compiler-failure
isolation is solid; AgentBriefService has no direct unit test but is well covered through
agent-map-mcp.test.ts.

@ynadge
ynadge force-pushed the review/agent-map-10-plan-tools branch from ad8c013 to 68f0d85 Compare September 5, 2026 11:57
@ynadge
ynadge force-pushed the review/agent-map-11-focused-briefs branch from 6766473 to 662edab Compare September 5, 2026 11:57
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — PR #830 (delta since 67664734)

Two commits landed: bf3dcf78 (reserved receipt namespace, recovery mapping, selection
indices) and 662edabb (two more public exports). No confidentiality issues in the new
prose — the changeset, docs/shared-build-plan.md and the new tests stay role-neutral.

Earlier findings — status

  • (1) derived brief requestId poisonable — fixed. harness-internal:brief: is now
    reserved via isCallerProjectRequestId on all four writer schemas that land in
    requestReceipts (brief refresh, plan apply, plan rebase, proposal batch), and the
    derived id moved into refreshAfterPlanMutation. I re-checked the residual collision I
    expected inside the reserved namespace — a no-op mutation reusing a plan versionId under
    a different map — and it is unreachable: apply only reports noOp when
    current.map === expectedMap (build-plan-service.ts:465) and rebase only when
    fromMap === toMap (:573), so the digest is stable per plan version.
  • (2) briefRefreshFailure mislabelling non-retryable errors — fixed. It now derives
    from errorResult, and agent-map-mcp.test.ts pins all five recovery outcomes.
  • (4) diagnostics pointing at the sorted index — fixed, with a regression test.
  • (3) public surface widened with no consumer — NOT fixed; widened further.
    662edabb adds PROJECT_AGENT_PROMPT_APPENDIX and projectAgentPromptAppendix to
    src/index.ts; nothing outside the package imports either (only relative imports in
    server/index.ts and tests), and the first pins the literal system-prompt text as a
    semver-stable constant. compileAgentBriefs is still a third alias for
    projectFocusedBriefs and the docs now call it "supported", making it permanent.
    Exporting AgentBriefService also hands external code refreshAfterPlanMutation, the
    trusted hook that writes into the reserved namespace the changeset says callers cannot
    occupy. The docs were corrected honestly (session controls "are not package exports"),
    but the changeset still does not list the two new prompt exports.

Verdict

The two agent-facing correctness bugs are genuinely fixed. Finding (3) stands and the
follow-up commit moved the wrong way — trim the exports to what a host actually needs
before this publishes.

@ynadge
ynadge force-pushed the review/agent-map-10-plan-tools branch from 68f0d85 to 143787a Compare September 5, 2026 12:17
@ynadge
ynadge force-pushed the review/agent-map-11-focused-briefs branch from 662edab to 74884b1 Compare September 5, 2026 12:17
Base automatically changed from review/agent-map-10-plan-tools to main September 6, 2026 22:21
@ynadge
ynadge merged commit ecfd937 into main Sep 6, 2026
2 checks passed
@ynadge
ynadge deleted the review/agent-map-11-focused-briefs branch September 6, 2026 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant